Add e2e tests for extraFields and consumeMagicCode#2394
Conversation
📝 WalkthroughWalkthroughAdded a new e2e test file validating authentication with extra fields ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View Vercel preview at instant-www-js-extra-fields-e2e-auth-jsv.vercel.app. |
b43fd36 to
875b254
Compare
875b254 to
9a25cb5
Compare
9a25cb5 to
8a83be9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts`:
- Around line 20-30: The test currently calls fetch and immediately does await
res.json() (using variables apiUrl, adminToken, appId, res, data) which hides
HTTP failures as JSON parse errors; update each such block (the POST to
`${apiUrl}/admin/magic_code` and the other occurrences around lines ~94 and
~118) to first assert the response status (e.g., check res.ok or specific status
codes) and if not OK throw or fail the test with a clear message including
res.status and await res.text() (or the parsed error body) before attempting
res.json(), so failures surface as explicit HTTP errors rather than parse
exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a145323e-0488-434e-b54b-5468b000d995
📒 Files selected for processing (2)
client/packages/core/__tests__/src/auth-extra-fields.e2e.test.tsclient/packages/version/src/version.ts
| const res = await fetch(`${apiUrl}/admin/magic_code`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'app-id': appId, | ||
| authorization: `Bearer ${adminToken}`, | ||
| }, | ||
| body: JSON.stringify({ email }), | ||
| }); | ||
| const data = await res.json(); | ||
| return data.code; |
There was a problem hiding this comment.
Add explicit HTTP status assertions before JSON parsing.
Line 20, Line 94, and Line 118 parse JSON unconditionally. If backend/auth/deploy is misconfigured, failures become noisy parse errors instead of clear request failures.
🔧 Suggested patch
async function generateMagicCode(
appId: string,
adminToken: string,
email: string,
): Promise<string> {
const res = await fetch(`${apiUrl}/admin/magic_code`, {
@@
body: JSON.stringify({ email }),
});
+ if (!res.ok) {
+ throw new Error(`/admin/magic_code failed (${res.status}): ${await res.text()}`);
+ }
const data = await res.json();
return data.code;
}
@@
const res = await fetch(
`${apiUrl}/admin/verify_magic_code?app_id=${appId}`,
{
@@
},
);
+ if (!res.ok) {
+ throw new Error(
+ `/admin/verify_magic_code failed (${res.status}): ${await res.text()}`,
+ );
+ }
const data = await res.json();
@@
const res2 = await fetch(
`${apiUrl}/admin/verify_magic_code?app_id=${appId}`,
{
@@
},
);
+ if (!res2.ok) {
+ throw new Error(
+ `/admin/verify_magic_code failed (${res2.status}): ${await res2.text()}`,
+ );
+ }
const data2 = await res2.json();Also applies to: 94-109, 118-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/packages/core/__tests__/src/auth-extra-fields.e2e.test.ts` around
lines 20 - 30, The test currently calls fetch and immediately does await
res.json() (using variables apiUrl, adminToken, appId, res, data) which hides
HTTP failures as JSON parse errors; update each such block (the POST to
`${apiUrl}/admin/magic_code` and the other occurrences around lines ~94 and
~118) to first assert the response status (e.g., check res.ok or specific status
codes) and if not OK throw or fail the test with a clear message including
res.status and await res.text() (or the parsed error body) before attempting
res.json(), so failures surface as explicit HTTP errors rather than parse
exceptions.
There was a problem hiding this comment.
Nitty / not-neccesary
There was a problem hiding this comment.
@nezaj, understood — acknowledged and moving on!
🧠 Learnings used
Learnt from: CR
Repo: instantdb/instant PR: 0
File: client/packages/create-instant-app/template/rules/AGENTS.md:0-0
Timestamp: 2026-03-20T22:27:09.580Z
Learning: Applies to client/packages/create-instant-app/template/rules/**/*.{ts,tsx,js,jsx} : Pass `extraFields` to sign-in methods to set custom properties on $users atomically during signup. Fields must be optional attributes on $users in schema.
Learnt from: dwwoelfel
Repo: instantdb/instant PR: 2329
File: client/packages/resumable-stream/__tests__/src/testing-stream.ts:6-13
Timestamp: 2026-02-26T20:42:24.553Z
Learning: In the instantdb/instant repository, test helper utilities under any __tests__ directories may implement simpler logic compared to production code. Apply this learning to review test utilities in client/packages/resumable-stream/__tests__/src/testing-stream.ts by prioritizing correctness for test scenarios while avoiding unnecessary production-grade edge-case handling. This guidance applies broadly to test helper files (TypeScript tests) and should not constrain production code paths.
I had these passing locally but they fail in CI since we need the backend to be deployed. Putting these up separately and will rebase against main once #2359 lands